Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle VLAN ranges in connection requests #240

Merged
merged 56 commits into from
Nov 15, 2024
Merged

Handle VLAN ranges in connection requests #240

merged 56 commits into from
Nov 15, 2024

Conversation

sajith
Copy link
Member

@sajith sajith commented Oct 28, 2024

Resolves #230.

@sajith sajith self-assigned this Oct 28, 2024
@sajith sajith marked this pull request as ready for review November 13, 2024 16:57
@sajith
Copy link
Member Author

sajith commented Nov 13, 2024

This works mostly. There are some edge cases I should address, and error handling needs more tests. I will address them in follow-up PRs.

Copy link
Collaborator

@YufengXin YufengXin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sajith Thanks for the enhancement, and the unittest cases are great.

Just one comment on the added codes (lines 962-992) in _find_common_vlan_on_link(): It requires that (1) the request vlan ranges are the same on the ingress port and egress port; (2) the same vlan range on all the intermediate ports on the path.

It's stricter than the single vlan case, where an arbitrary vlan can be picked on the intermediate inter-oxp link.

Do I understand it correctly?

@YufengXin
Copy link
Collaborator

@sajith I changed one line for an unrelated issue: Internal server error while submitting a request with VLAN out of the allowed range atlanticwave-sdx/sdx-controller#356.

@sajith sajith merged commit 00b09cb into main Nov 15, 2024
6 checks passed
@sajith sajith deleted the 230.vlan-range branch November 15, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

Handle connection request vlan range
3 participants